Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FPGA prototyping platform VCS simulation #871

Closed
wants to merge 29 commits into from
Closed

FPGA prototyping platform VCS simulation #871

wants to merge 29 commits into from

Conversation

jamesdunn
Copy link
Member

Related issue: #820

Type of change: new feature

Impact: software change

Release Notes

This PR contains a draft of FPGA prototyping sim support. It currently targets Arty, but I have tried to generalize the flow such that it should be able to target the VCU118 with minor changes.

The sim binary builds, but exhibits the same behavior described in #820, so please do not merge. However, I wanted to get in a PR so that the behavior can be reproduced with a make target. In that issue, the sim binary is missing the program-as-argument functionality given in testchip_tsi.cc. David suggested looking into +permissive_on/off flags, which I will do next.

To summarize what this PR contains, I have added a sim-fpga target in fpga/Makefile that runs a .tcl script, /fpga/scripts/generate_vcs_collateral.tcl, to generate Vivado behavioral sim collateral for VCS. It then runs through the VCS mixed-language flow, analyzing the Verilog and VHDL, then elaborating the simulation binary.

I will definitely do a lot of cleanup before merging this. Partcularly de-duplication between this Makefile and some things in vcs.mk that were hard-coded. Also figuring out a cleaner way to bring in an FPGA-specific testdriver, which for now uses an additional case in generators/utilities/src/main/scala/Simulator.scala for a new sim_name.

To run:

./scripts/init-fpga.sh
cd fpga
make SUB_PROJECT=artysim sim-fpga

James Dunn and others added 10 commits March 15, 2021 11:45
…permanent fix since we do want this tied off for hardware.
… of adding a case in Simulator.scala for a new simulation variable, arty. This can change later.
…cies with includes in build.sbt. Will be changed later.
…mulation libraries. Next, sim-fpga target will include the VCS MX flow commands.
… described in issue, but it can now be replicated.
@jamesdunn jamesdunn requested a review from abejgonzalez April 29, 2021 23:10
@jamesdunn jamesdunn changed the title FPGA Prototyping Platform VCS simulation FPGA prototyping platform VCS simulation Apr 29, 2021
@jamesdunn jamesdunn linked an issue Apr 29, 2021 that may be closed by this pull request
Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, a good start. It might be worth it to add a Makefile check for Vivado versions (what Vivado version are you using?).

fpga/Makefile Outdated Show resolved Hide resolved
fpga/Makefile Outdated Show resolved Hide resolved
fpga/Makefile Outdated Show resolved Hide resolved
fpga/Makefile Outdated Show resolved Hide resolved
fpga/Makefile Show resolved Hide resolved
fpga/src/main/scala/arty/Configs.scala Outdated Show resolved Hide resolved
fpga/src/main/scala/arty/TestHarness.scala Outdated Show resolved Hide resolved
@jamesdunn
Copy link
Member Author

Overall, a good start. It might be worth it to add a Makefile check for Vivado versions (what Vivado version are you using?).

Thanks for your review and comments, Abe. I have added a Makefile variable VIVADO_VERSION that gets the version. I am currently using v2019.2. I anticipate other versions being compatible, but I agree that it's good to support only one version and do a check. Do you recommend erroring out of the build on a version mismatch? If so, what make target should that be under? The bitstream generation and simulation targets are independent of one another, so I could put it under both.

@@ -14,6 +14,9 @@ sim_name := none
#########################################################################################
# include shared variables
#########################################################################################

VIVADO_VERSION := $(shell vivado -version | head -1 | grep -o -P '(?<=\s).*(?=\s)')
Copy link
Contributor

@abejgonzalez abejgonzalez May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Vivado isn't on the path this pollutes the terminal output. Maybe pipe stderr to /dev/null. Also, where is this checked/used?

@jamesdunn

This comment has been minimized.

@jamesdunn

This comment has been minimized.

@jamesdunn

This comment has been minimized.

@jamesdunn

This comment has been minimized.

@jamesdunn

This comment has been minimized.

end

`MODEL testHarness(
.CLK100MHZ(clock),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can avoid duplicating this file by creating a new Shell called something like ChipyardArtyShell which 1 instantiates the current ArtyShell but also renames the CLK100MHz -> clock and ck_rst -> reset (and also does the negation within it). Maybe this can also be done w/ the fancier diplomatic Arty100TShell if we don't want to hack around w/ this basic shell?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it's best to avoid keeping excess collateral files like this around. Seems like it would be a good solution to create a new shell and rename to conform to the stock CY TestDriver. I'll look at that but will want to make sure that nothing in fpga-shells depends on these names. I haven't looked as much at the diplomatic Arty100TShell but will do so. I think it would be best to make Arty diplomatic to bring it in line with the VCU118 shell.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, if the test driver is the same and the tcl scripts are generalized... then I should be able to port this work to test the VCU118. Then if that works, I can probably add both of these tests into CI.

fpga/scripts/generate_vcs_collateral.tcl Outdated Show resolved Hide resolved
fpga/src/main/scala/arty/Configs.scala Outdated Show resolved Hide resolved
fpga/src/main/scala/arty/HarnessBinders.scala Outdated Show resolved Hide resolved
@jamesdunn
Copy link
Member Author

This is now working. Users with Vivado and VCS in their paths can run

make SUB_PROJECT=artysim sim-fpga

in the fpga directory, and a sim binary should be built in generated-src/[project name]

You can then pass a program to the generated binary as with the standard RTL sim flow.

@jamesdunn jamesdunn changed the base branch from master to dev October 13, 2021 20:42
@sagark sagark added this to the unknown-future milestone Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prototyping Harness Sim Testbench
4 participants